SK-2496: extract hard coded values to constants #229
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| response_object = { | ||
| "token": actual_token, | ||
| "signed_token": signed_token | ||
| ResponseField.TOKEN: actual_token, |
Check failure
Code scanning / Semgrep OSS
Semgrep Finding: semgreprules.check-sensitive-info Error
There was a problem hiding this comment.
Skyflow data token (non-sensitive, not an auth token)
| "token": record.token, | ||
| "error": record.error, | ||
| "request_id": request_id | ||
| ResponseField.TOKEN: record.token, |
Check failure
Code scanning / Semgrep OSS
Semgrep Finding: semgreprules.check-sensitive-info Error
There was a problem hiding this comment.
Skyflow data token (non-sensitive, not an auth token)
| "token": record.token, | ||
| "value": record.value, | ||
| "type": value_type | ||
| ResponseField.TOKEN: record.token, |
Check failure
Code scanning / Semgrep OSS
Semgrep Finding: semgreprules.check-sensitive-info Error
There was a problem hiding this comment.
Skyflow data token (non-sensitive, not an auth token)
| def parse_tokenize_response(api_response: V1TokenizeResponse): | ||
| tokenize_response = TokenizeResponse() | ||
| tokenized_fields = [{"token": record.token} for record in api_response.records] | ||
| tokenized_fields = [{ResponseField.TOKEN: record.token} for record in api_response.records] |
Check failure
Code scanning / Semgrep OSS
Semgrep Finding: semgreprules.check-sensitive-info Error
There was a problem hiding this comment.
Skyflow data token (non-sensitive, not an auth token)
| DETAILS = 'details' | ||
| MESSAGE = 'message' | ||
| ERROR_FROM_CLIENT = 'error_from_client' | ||
| TOKEN = 'token' |
Check failure
Code scanning / Semgrep OSS
Semgrep Finding: semgreprules.check-sensitive-info Error
There was a problem hiding this comment.
Skyflow data token (non-sensitive, not an auth token)
| KEY_ID = 'keyID' | ||
| TOKEN_URI = 'tokenURI' | ||
| CREDENTIALS_STRING = 'credentials_string' | ||
| API_KEY = 'api_key' |
Check failure
Code scanning / Semgrep OSS
Semgrep Finding: semgreprules.check-sensitive-info Error
There was a problem hiding this comment.
Skyflow configuration field (non-sensitive, not a secret)
| TOKEN_URI = 'tokenURI' | ||
| CREDENTIALS_STRING = 'credentials_string' | ||
| API_KEY = 'api_key' | ||
| TOKEN = 'token' |
Check failure
Code scanning / Semgrep OSS
Semgrep Finding: semgreprules.check-sensitive-info Error
There was a problem hiding this comment.
Skyflow data token (non-sensitive, not an auth token)
| V1DetokenizeRecordRequest( | ||
| token=item.get('token'), | ||
| redaction=item.get('redaction', RedactionType.DEFAULT) | ||
| token=item.get(ResponseField.TOKEN), |
Check failure
Code scanning / Semgrep OSS
Semgrep Finding: semgreprules.check-sensitive-info Error
There was a problem hiding this comment.
Skyflow data token (non-sensitive, not an auth token)
There was a problem hiding this comment.
Pull request overview
This PR refactors the Python SDK v2 to replace hardcoded string literals with well-defined constants, improving code maintainability and consistency. This follows a linting audit similar to the Node SDK refactor.
Changes:
- Introduced new constant classes in
skyflow/utils/constants.pyfor various domains (HTTP headers, file extensions, JWT fields, etc.) - Replaced hardcoded strings throughout the codebase with references to these constants
- Updated imports across multiple modules to use the new constants
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| skyflow/utils/constants.py | Added comprehensive constant definitions organized into domain-specific classes |
| skyflow/vault/controller/_vault.py | Replaced hardcoded field names with ResponseField and RequestParameter constants |
| skyflow/vault/controller/_detect.py | Replaced hardcoded status strings, file extensions, and encoding types with constants |
| skyflow/vault/controller/_connections.py | Replaced hardcoded header names with constants |
| skyflow/vault/client/client.py | Replaced hardcoded config/credential field names with constants |
| skyflow/utils/validations/_validations.py | Replaced hardcoded field names and API key validation literals with constants |
| skyflow/utils/logger/_log_helpers.py | Replaced hardcoded log field names with ResponseField constants |
| skyflow/utils/_utils.py | Replaced hardcoded HTTP headers, content types, and response fields with constants |
| skyflow/utils/_skyflow_messages.py | Added new error message constants |
| skyflow/service_account/_utils.py | Replaced hardcoded JWT fields and credential fields with constants |
| skyflow/client/skyflow.py | Replaced hardcoded option field names with OptionField constants |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if status == 'IN_PROGRESS': | ||
| if status == DetectStatus.IN_PROGRESS: | ||
| if current_wait_time >= max_wait_time: | ||
| return DeidentifyFileResponse(run_id=run_id, status='IN_PROGRESS') |
There was a problem hiding this comment.
The hardcoded string 'IN_PROGRESS' should be replaced with the constant DetectStatus.IN_PROGRESS for consistency with line 68.
| return DeidentifyFileResponse(run_id=run_id, status='IN_PROGRESS') | |
| return DeidentifyFileResponse(run_id=run_id, status=DetectStatus.IN_PROGRESS) |
| ) | ||
| if response.data.status == 'IN_PROGRESS': | ||
| if response.data.status == DetectStatus.IN_PROGRESS: | ||
| parsed_response = self.__parse_deidentify_file_response(DeidentifyFileResponse(run_id=run_id, status='IN_PROGRESS')) |
There was a problem hiding this comment.
The hardcoded string 'IN_PROGRESS' should be replaced with the constant DetectStatus.IN_PROGRESS for consistency with the rest of the refactoring.
| parsed_response = self.__parse_deidentify_file_response(DeidentifyFileResponse(run_id=run_id, status='IN_PROGRESS')) | |
| parsed_response = self.__parse_deidentify_file_response(DeidentifyFileResponse(run_id=run_id, status=DetectStatus.IN_PROGRESS)) |
Why:
Following the Node SDK linting audit and refactor, this ticket focuses on applying the same linting standards and best practices across Python SDK v2.
The goal is to ensure consistent use of constants over hardcoded values across Python SDK v2, improving maintainability and readability.
Goal:
Update linting rules to discourage hardcoded literals where constants should be used.
Refactor existing code to replace hardcoded values with well-defined constants.